-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add option to not use SmartnoiseSQL #77
Conversation
I noticed yesterday that what I had written in this PR didn't work. I think I fixed it, but this means the tests weren't hitting the new code. I still need to figure out why, and improve the tests. |
|
||
else: | ||
|
||
def execute_query(conn: Any, stat_data: Dict[str, Any]) -> Any: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we should type the return as List[List] (or Iterable[Iterable]) or similar?
def execute_query(conn: Any, stat_data: Dict[str, Any]) -> Any: | |
def execute_query(conn: Any, stat_data: Dict[str, Any]) -> List[List]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is we need to change the type signature of execute_query
in both branches of the if statement to keep mypy happy (All conditional function variants must have identical signatures
), and mypy doesn't know what the return type of SNSQL's private_reader.execute
is.
With this PR, you can now set a top-level setting in
config.yaml
to sayuse-smartnoise-sql: False
, and the queries will be run raw, without SNSQL to noise them.I also threw in a couple of unrelated tiny changes because I'm lazy to make one line PRs.
Closes #75